fix(cli): move top-level command routing into folders#113
fix(cli): move top-level command routing into folders#113Agent-Hellboy merged 4 commits intomainfrom
Conversation
Keep CLI behavior unchanged while moving top-level Cobra routing into the foldered internal/cli command packages where the shared boundary is clean. Also document that agents must work from branches and PRs instead of pushing directly to main.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the CLI by moving command definitions from the main internal/cli package into specific sub-packages, necessitating the exportation of various internal functions and methods. Documentation and agent guidelines were also updated. Feedback highlights opportunities to reduce code duplication by removing redundant routing logic and constants from the base package. A suggestion was also made to improve argument validation for the server creation command when using file-based inputs.
| grantResource = "mcpaccessgrant" | ||
| sessionResource = "mcpagentsession" |
There was a problem hiding this comment.
These resource name constants are duplicated from internal/cli/access.go. To ensure consistency and avoid potential runtime errors if the strings diverge, consider exporting the constants from the cli package (e.g., AccessGrantResource) and referencing them here.
| } | ||
|
|
||
| // NewWithManager returns the access command using the provided manager. | ||
| func NewWithManager(mgr *cli.AccessManager) *cobra.Command { |
There was a problem hiding this comment.
The command routing logic here is duplicated from internal/cli/access.go. To adhere to the PR's goal of moving routing into folders, the Cobra command definitions should be removed from the base cli package to ensure a single source of truth for command structures.
| } | ||
|
|
||
| // NewWithManager returns the server command using the provided manager. | ||
| func NewWithManager(mgr *cli.ServerManager) *cobra.Command { |
There was a problem hiding this comment.
This function and the associated subcommand definitions are duplicated from internal/cli/server.go. To maintain code quality and avoid divergence, the routing logic should be removed from the base cli package now that it has been moved here.
| Use: "create [name]", | ||
| Short: "Create an MCP server", | ||
| Long: "Create a new MCP server deployment", | ||
| Args: cobra.ExactArgs(1), |
There was a problem hiding this comment.
The ExactArgs(1) constraint causes the command to fail when the --file flag is used without a positional argument, even though the name argument is ignored in that case. A dynamic validator would provide a better user experience.
Args: func(cmd *cobra.Command, args []string) error {
if file != "" {
return cobra.NoArgs(cmd, args)
}
return cobra.ExactArgs(1)(cmd, args)
},
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #113 +/- ##
==========================================
- Coverage 58.01% 51.64% -6.38%
==========================================
Files 70 72 +2
Lines 10700 10679 -21
==========================================
- Hits 6208 5515 -693
- Misses 3897 4580 +683
+ Partials 595 584 -11
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Build the shared CLI dependencies once in internal/cli and pass a runtime facade into the foldered command packages. Keep Cobra trees and command-local orchestration in the folders while shared managers and helpers stay in internal/cli.
Move the auth command manager, login flow, verification helpers, and tests into internal/cli/auth so the subcommand folder owns its command-specific behavior. Keep only the shared platform base-URL normalization helper in internal/cli for reuse by platform_client.
|
@codex please review |
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Keep CLI behavior unchanged while moving top-level Cobra routing into the foldered internal/cli command packages where the shared boundary is clean. Also document that agents must work from branches and PRs instead of pushing directly to main.